Conversation
✅ Deploy Preview for ap-template-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…ontrols Signed-off-by: adarshh <adarsh347k@gmail.com>
…nd chat controls Signed-off-by: adarshh <adarsh347k@gmail.com>
… test file Signed-off-by: adarshh <adarsh347k@gmail.com>
9b6c333 to
a62d846
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses Issue #768 by improving keyboard accessibility across several UI components in the Template Playground. It adds onKeyDown handlers, role="button", tabIndex={0}, aria-label, and aria-pressed attributes to interactive elements that were previously only mouse-operable.
Changes:
PlaygroundSidebar.tsx: AddsonKeyDownhandlers (Enter/Space) to top and bottom nav items, withpreventDefault()to avoid scroll on Space.FullScreenModal.tsx: Addsrole="button",tabIndex={0},aria-label, andonKeyDownto the fullscreen trigger icon.AIChatPanel.tsx: Addsrole="button",tabIndex={0},aria-label,aria-pressed, andonKeyDownto the three context toggle divs.KeyboardAccessibility.test.tsx(new): Unit tests for keyboard activation of sidebar nav items.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/components/PlaygroundSidebar.tsx |
Adds onKeyDown handlers to top and bottom nav item wrappers |
src/components/FullScreenModal.tsx |
Adds keyboard accessibility attributes and handler to the MdFullscreen icon |
src/components/AIChatPanel.tsx |
Adds keyboard accessibility attributes and handlers to the three context toggle <div> elements |
src/tests/components/KeyboardAccessibility.test.tsx |
New test file covering keyboard activation of sidebar items |
Comments suppressed due to low confidence (1)
src/components/PlaygroundSidebar.tsx:184
- The "Fullscreen" nav item wrapper (
div[role="button"]) has noonClickhandler because theFullscreenentry innavTop(line 127) omits theonClickproperty. As a result, when a keyboard user focuses the outer wrapper and presses Enter or Space,onClick?.()is a no-op — nothing happens.
Meanwhile, FullScreenModal renders its own MdFullscreen icon that is now also role="button" with tabIndex={0}. This creates two nested interactive elements for the same action, which is invalid per WCAG 4.1.3 (nested interactive elements). A screen reader and keyboard user will encounter an outer "Fullscreen" button that does nothing and an inner "Open fullscreen preview" button that works.
To fix this, the Fullscreen nav item in navTop should either:
- Provide an
onClickthat delegates to the fullscreen modal's open action (lifting state up), or - Remove
tabIndex={0}androle="button"from theFullScreenModal'sMdFullscreenicon (since the outer wrapper now handles keyboard), and instead pass the open handler through props.
Currently, the outer nav item wrapper for "Fullscreen" provides no keyboard functionality despite being marked as a button.
<div
role="button"
aria-label={title}
tabIndex={0}
onClick={onClick}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
onClick?.();
}
}}
className={`group playground-sidebar-nav-item ${
active ? 'playground-sidebar-nav-item-active' : 'playground-sidebar-nav-item-inactive'
} tour-${title.toLowerCase().replace(' ', '-')}`}
>
{component ? (
<div className="playground-sidebar-nav-item-icon-container">
{component}
</div>
) : Icon ? (
<Icon size={20} />
) : null}
<span className="playground-sidebar-nav-item-title">{title}</span>
</div>
| role="button" | ||
| tabIndex={0} | ||
| aria-label="Open fullscreen preview" | ||
| style={{ cursor: "pointer" }} | ||
| onClick={() => setOpen(true)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| setOpen(true); | ||
| } | ||
| }} |
There was a problem hiding this comment.
The FullScreenModal's MdFullscreen icon now has role="button", tabIndex={0}, and aria-label attributes, but it is rendered inside the "Fullscreen" nav item container in PlaygroundSidebar, which also has role="button" and tabIndex={0}. This creates a nested interactive elements structure — a role="button" element inside another role="button" — which is invalid per the ARIA spec (interactive controls cannot be nested inside each other). Screen readers and assistive technologies may handle this unpredictably.
| role="button" | |
| tabIndex={0} | |
| aria-label="Open fullscreen preview" | |
| style={{ cursor: "pointer" }} | |
| onClick={() => setOpen(true)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| setOpen(true); | |
| } | |
| }} | |
| style={{ cursor: "pointer" }} | |
| onClick={() => setOpen(true)} |
| describe("Keyboard Accessibility", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| describe("PlaygroundSidebar - Top Nav", () => { | ||
| it("activates Editor button on Enter key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const editorButton = screen.getByRole("button", { name: /Editor/i }); | ||
| fireEvent.keyDown(editorButton, { key: "Enter" }); | ||
| expect(mockSetEditorsVisible).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("activates Editor button on Space key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const editorButton = screen.getByRole("button", { name: /Editor/i }); | ||
| fireEvent.keyDown(editorButton, { key: " " }); | ||
| expect(mockSetEditorsVisible).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("does not activate Editor button on other keys", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const editorButton = screen.getByRole("button", { name: /Editor/i }); | ||
| fireEvent.keyDown(editorButton, { key: "Tab" }); | ||
| expect(mockSetEditorsVisible).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("activates Preview button on Enter key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const previewButton = screen.getByRole("button", { name: /Preview/i }); | ||
| fireEvent.keyDown(previewButton, { key: "Enter" }); | ||
| expect(mockSetPreviewVisible).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("activates Problems button on Space key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const problemsButton = screen.getByRole("button", { name: /Problems/i }); | ||
| fireEvent.keyDown(problemsButton, { key: " " }); | ||
| expect(mockSetProblemPanelVisible).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("activates AI Assistant button on Enter key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const aiButton = screen.getByRole("button", { name: /AI Assistant/i }); | ||
| fireEvent.keyDown(aiButton, { key: "Enter" }); | ||
| expect(mockSetAIChatOpen).toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("PlaygroundSidebar - Bottom Nav", () => { | ||
| it("activates Share button on Enter key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const shareButton = screen.getByRole("button", { name: /Share/i }); | ||
| fireEvent.keyDown(shareButton, { key: "Enter" }); | ||
| // Share calls handleShare which is async, just verify no error | ||
| expect(shareButton).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("activates Settings button on Space key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const settingsButton = screen.getByRole("button", { name: /Settings/i }); | ||
| fireEvent.keyDown(settingsButton, { key: " " }); | ||
| expect(mockSetSettingsOpen).toHaveBeenCalledWith(true); | ||
| }); | ||
|
|
||
| it("activates Start Tour button on Enter key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const tourButton = screen.getByRole("button", { name: /Start Tour/i }); | ||
| fireEvent.keyDown(tourButton, { key: "Enter" }); | ||
| expect(tourButton).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("PlaygroundSidebar - focusability", () => { | ||
| it("all nav items have tabIndex=0 for keyboard focus", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const buttons = screen.getAllByRole("button"); | ||
| buttons.forEach((button) => { | ||
| expect(button).toHaveAttribute("tabindex", "0"); | ||
| }); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The keyboard accessibility changes for AIChatPanel (context toggle buttons) and FullScreenModal (fullscreen trigger) are not covered by any unit tests. The new test file only tests PlaygroundSidebar keyboard accessibility. Given that the PR description states "Vital features and changes captured in unit and/or integration tests" is unchecked in the author checklist, and that the codebase has unit tests for components like SettingsModal and PlaygroundSidebar, tests for the onKeyDown handlers in AIChatPanel and FullScreenModal should be added.
| it("activates Share button on Enter key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const shareButton = screen.getByRole("button", { name: /Share/i }); | ||
| fireEvent.keyDown(shareButton, { key: "Enter" }); | ||
| // Share calls handleShare which is async, just verify no error | ||
| expect(shareButton).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("activates Settings button on Space key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const settingsButton = screen.getByRole("button", { name: /Settings/i }); | ||
| fireEvent.keyDown(settingsButton, { key: " " }); | ||
| expect(mockSetSettingsOpen).toHaveBeenCalledWith(true); | ||
| }); | ||
|
|
||
| it("activates Start Tour button on Enter key", () => { | ||
| render(<PlaygroundSidebar />); | ||
| const tourButton = screen.getByRole("button", { name: /Start Tour/i }); | ||
| fireEvent.keyDown(tourButton, { key: "Enter" }); | ||
| expect(tourButton).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
The tests for "Share" (lines 92-98) and "Start Tour" (lines 107-112) fire keyDown events but their assertions only verify that the button element is in the document (expect(button).toBeInTheDocument()). This does not actually verify that the keyboard handler was triggered or that any action occurred. Such tests will always pass regardless of whether the onKeyDown handler works or not.
For the "Share" test, navigator.clipboard.writeText could be mocked to verify it was called. For the "Start Tour" test, the Tour mock is already set up and Tour.start could be asserted to have been called (vi.mock("../../components/Tour", ...)). At minimum, the comment "Share calls handleShare which is async, just verify no error" acknowledges this but the same weak pattern is applied to the Tour test without explanation.
sanketshevkar
left a comment
There was a problem hiding this comment.
@adarshh347 thanks for raising this PR. Is it possible for you to please include a a11y report before and after your changes? You can use axe devtool to generate the report
I'd suggest if there are multiple issues identified and changes spanning across a large number of files. Its better to divide the tasks down to small issues and raise PRs separately.
feat(a11y): enable keyboard accessibility for sidebar, fullscreen, and chat controls
Closes #
This PR improves keyboard accessibility across multiple interactive components in the Playground.
Several UI controls were implemented using non-semantic
<div>elements or raw icons withonClickhandlers only. While they functioned correctly with mouse interaction, they were not fully accessible to keyboard users or assistive technologies.This update ensures all interactive elements can be focused via
Taband activated usingEnterorSpace, improving overall WCAG alignment and usability.Changes
onKeyDownhandlers to PlaygroundSidebar navigation items to supportEnterandSpaceactivationrole="button"andtabIndex={0}where necessary for semantic clarityaria-labelto the fullscreen icon inFullScreenModalrole="button",tabIndex={0},aria-label, andaria-pressedto AIChatPanel context togglespreventDefault()is used to avoid unintended scroll behavior when pressing SpaceFlags
Screenshots or Video
N/A – No visual UI changes.
Functionality verified through keyboard navigation testing.
Related Issues
Author Checklist
--signoffoptionmainfromfork:<branchname>